-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Paywalls: Open up paywalls from deeplinks #4285
base: main
Are you sure you want to change the base?
Conversation
"appPolicies" : { | ||
"eula" : "", | ||
"policies" : [ | ||
{ | ||
"locale" : "en_US", | ||
"policyText" : "", | ||
"policyURL" : "" | ||
} | ||
] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh interesting, the stuff in this file just gets updated automatically when you open up Xcode 16. Will revert since it's not actually part of the PR
@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *) | ||
@available(macOS, unavailable, message: "RevenueCatUI does not support macOS yet") | ||
@available(tvOS, unavailable, message: "RevenueCatUI does not support tvOS yet") | ||
struct RevenueCatDeeplinkHandlerView<Content: View>: View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to get this through the finish line, we really need to:
- settle on the exact format. I used
/paywall
in the examples but I'm not even enforcing it, mayberevenuecatui
is more appropriate? - clean up error handling and logging
- add tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly I think we should validate the deep link path at least, to try to avoid all links opening the paywall inadvertently. Looks great otherwise!
|
||
var body: some View { | ||
content | ||
.onOpenURL { url in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we would like to also support UIKit apps... but I guess that can come in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I figured for UIKit we'd basically do the appDelegate + present from root view controller path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think just adding a method that can be called by a developer in the app delegate method would work 👍
@@ -44,6 +44,7 @@ struct AppContentView: View { | |||
} | |||
#endif | |||
} | |||
.handleRevenueCatDeeplinks() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to do here but I must say, I'm still not used to having these things as modifiers of a view, seems like a very weird API to me 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 what would a more natural API look like to you as a dev? I'm open to anything as long as its easy to do. I know others do the setup in the app delegate, but the industry seems to be moving away from that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this is the way it's supposed to be in SwiftUI (like the onOpenURL
API we're using). Just seems very weird to tie this behavior to a view IMO... So yeah, I think this is the most SwiftUI solution for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I feel you Toni haha, but indeed this seems to be the SwiftUI way. In Compose it would be more natural to add a route to a navigation graph. The downside of that is that there are multiple ways to build such a graph, as mentioned on Slack.
var body: some View { | ||
Group { | ||
if let offering = offering { | ||
PaywallView(offering: offering) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it, we might also want to offer a way to not display the paywall if the user has an entitlement (passing a requiredEntitlementIdentifier
). But this could also be a separate PR I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that's a good point. It could be done as an extra query param in the deeplink too
Quick-n-dirty PoC to open up paywalls from a deeplink.
Usage:
In a SwiftUIView, just add a modifier:
And we take it from there! You will have to set up deeplinks for your app, though, obviously.
Then, once set up, you can use deeplinks like:
Also added them to paywallsTester for easy testing.
Loom demo: https://www.loom.com/share/bd45fc768008462e97b1601ce07f1104